verdi process show: order called by ctime and use process label
#4407
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #4407 +/- ##
===========================================
+ Coverage 79.22% 79.23% +0.01%
===========================================
Files 475 475
Lines 34826 34826
===========================================
+ Hits 27589 27591 +2
+ Misses 7237 7235 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
| if nodes_caller: | ||
| result += '\n' + format_flat_links( | ||
| sorted(nodes_caller.all(), key=lambda x: x.node.ctime), headers=['Caller', 'PK', 'Type'] | ||
| ) |
There was a problem hiding this comment.
(oops, I think I forgot to add this to the review, I'll just leave it here)
This "if-path" seems not to be covered by the tests; I think the easiest way to include it would be by either adding a cli-invocation on one of the calcjob nodes + respective checks, or by adding a call link between the workchains and just adding a check after the already existing cli-invoke (I woult think the second one is the most "economical one", but I'm not sure if you had a reason for creating those two workchains and leaving one of those disconected).
Also, tangential question: a process will either have one caller or none at all, right? Why do we write this here like there could be many callers?
There was a problem hiding this comment.
I woult think the second one is the most "economical one", but I'm not sure if you had a reason for creating those two workchains and leaving one of those disconected
No particular reason to leave it unconnected just that I only needed one of those two to do the test. I will add the second one as the caller of the first one and add the test.
Also, tangential question: a process will either have one caller or none at all, right? Why do we write this here like there could be many callers?
Absolutely correct. There is no specific reason other than that it is just easier to write and understand compared to the nodes_called.
The command was showing the called subprocesses in a random order and used the node type, which is often uninformative. For example, all workchains are always shown as `WorkChainNode`. By using the process label instead, which is more specific, and ordering the called nodes by creation time, the list gives a more natural overview of the order in which the subprocesses were called.
sphuber
force-pushed
the
fix/4406/process-show-call-links
branch
from
3a45ff7
to
9d29ccd
Compare
Fixes #4406
The command was showing the called subprocesses in a random order and
used the node type, which is often uninformative. For example, all
workchains are always shown as
WorkChainNode. By using the processlabel instead, which is more specific, and ordering the called nodes by
creation time, the list gives a more natural overview of the order in
which the subprocesses were called.